Skip to content

Fix HELLO command handler for empty command input #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Feb 22, 2023
Merged

Fix HELLO command handler for empty command input #33

merged 2 commits into from
Feb 22, 2023

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Feb 22, 2023

Previously the discovery handler would not gracefully handle an empty HELLO command:

$ ./dummy-discovery
HELLO
panic: runtime error: slice bounds out of range [6:5]

goroutine 1 [running]:
github.com/arduino/pluggable-discovery-protocol-handler/v2.(*Server).Run(0xc00002c1e0, {0x5147c8?, 0xc000012010?}, {0x5147e8?, 0xc000012018?})
        /home/megabug/Workspace/pluggable-discovery-protocol-handler/discovery.go:135 +0x5a5
main.main()
        /home/megabug/Workspace/pluggable-discovery-protocol-handler/dummy-discovery/main.go:44 +0x88
$

After this patch, it is handled correctly:

$ ./dummy-discovery 
HELLO
{
  "eventType": "hello",
  "message": "Invalid HELLO command",
  "error": true
}
QUIT
{
  "eventType": "quit",
  "message": "OK"
}
$

Fix #32

@cmaglie cmaglie self-assigned this Feb 22, 2023
@cmaglie cmaglie added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Feb 22, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2023 •
edited
Loading

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (71b0ce2) compared to base (69bcf9a).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@          Coverage Diff          @@
##            main     #33   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          4       4           
  Lines        255     259    +4     
=====================================
- Misses       255     259    +4
Flag Coverage Δ
unit 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
discovery.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cmaglie cmaglie mentioned this pull request Feb 22, 2023
Closed
3 tasks
per1234
per1234 approved these changes Feb 22, 2023
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Cristian!

@cmaglie cmaglie merged commit 64ac0f7 into main Feb 22, 2023
@cmaglie cmaglie deleted the fix_hello_handling branch February 22, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 per1234 approved these changes

@Bikappa Bikappa Awaiting requested review from Bikappa

@MatteoPologruto MatteoPologruto Awaiting requested review from MatteoPologruto

@umbynos umbynos Awaiting requested review from umbynos

Assignees

@cmaglie cmaglie

Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Panic when no arguments passed via HELLO command
3 participants
@cmaglie @codecov-commenter @per1234